changes to issue #1558 regarding failed Jest test cases#1577
changes to issue #1558 regarding failed Jest test cases#1577binkmywink wants to merge 1 commit intotl-its-umich-edu:masterfrom
Conversation
zqian
changed the title
| @@ -1,4 +1,4 @@ | |||
| import * as d3 from 'd3' | |||
| import * as d3 from 'd3/dist/d3.min' | |||
There was a problem hiding this comment.
I don't feel we import things from dist folder. This change effects the View. why are you doing this?
There was a problem hiding this comment.
None of the imports in the project we use dist in path while importing. This needs to be changed.
There was a problem hiding this comment.
On my end I believe there is a parsing issue when I leave the import line as import * as d3 from 'd3'
There was a problem hiding this comment.
As per my understanding we cannot have an import from dist folder. That mean something in the way we process this needs to change.
There was a problem hiding this comment.
Sounds good, I'll look into what could be changed
| ["@babel/preset-react",{"runtime": "automatic"}]] | ||
| [ | ||
| "@babel/preset-env", | ||
| { |
There was a problem hiding this comment.
I would like to keep this format same as before. What was the reason for re-formatting this?
| // // The output is always the same. | ||
| // return 'cssTransform'; | ||
| // } | ||
| // }; |
There was a problem hiding this comment.
You should always remove commented code , if not used
|
Can you email me or share your Uniqname? we are keeping track of students from EECS course who are contributing to our repo |
pushyamig
left a comment
pushyamig
left a comment
There was a problem hiding this comment.
I have commented on code and I felt needs change
|
@jxiao21 I suggest if you are assigned with this issue, you either have to take these change into you branch and have initial commit from contributor as below.
let me know if you have any question. You might have to resolve some conflict/rebase since the code needs to in sync latest from master |
|
Forgot to comment this yesterday but right now I may start it from scratch and use the PR as a reference since the current PR is outdated and I'm failing some of the tests with the current changes. Would that still be that would be done with a diff of the current PR? |
However you want to approach is is fine. If your code changes are adding on from this contributor change. Then have a commit with her name. But that could be added once you fixed this issue (you could do something like this But as of now you can work on fixing the issue and not worry committing with author name. I can help with that. |
3 participants
I was able to make all the tests work by rewriting the CSSTransform file and updating the Jest version in package.json (this can also be done by running
npm update jest. I used babelnpm install babel-jest --save-devto reconfigure some files, as I believe that was causing some of the tests to fail. The main goal was to configure Jest for ECMAScript Modules, as the issue came from trying to import d3 using ECMAScript Module syntax (import * as d3 from 'd3') in the Histogram test. Using the minified version of d3 resolved this issue. In addition to updating snapshots before running backend tests, I was able to compile all the tests successfully.